Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add block_while! macro that blocks while another expression evaluates to true #18

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

kellerkindt
Copy link

@kellerkindt kellerkindt commented Jun 30, 2019

I am using this macro derivative for an private project for a while now and decided to see whether others (like you) can make use of it as well. Basically it adds a condition to block! so that it does not block indefinitely. This allows code like this:

// ...
        let time = || info.uptime_ms();
        let timeout = time() + 500;
        let in_time = || timeout > time();

        block_while!(in_time(), tx.write(START_BYTE))
            .and(block_while!(in_time(), tx.write(0x01)))
            .and(block_while!(in_time(), tx.write(COMMAND)))
            .and(block_while!(in_time(), tx.write(0x00)))
            .and(block_while!(in_time(), tx.write(0x00)))
            .and(block_while!(in_time(), tx.write(0x00)))
            .and(block_while!(in_time(), tx.write(0x00)))
            .and(block_while!(in_time(), tx.write(0x00)))
            .and(block_while!(in_time(), tx.write(0x79)))
            .map_err(drop)
            .and_then(|_| {
                Ok([
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                ])
            })
            . // ...
// ...

The operation is still blocking but in this case has a timeout for the whole operation.

block_while! blocks as for the operation to finish as long as the expression $c evaluates to true.
@eldruin
Copy link
Member

eldruin commented Jun 30, 2020

This sounds fine to me. However it has been lying around for quite a while.
Does anybody else have concerns about it @rust-embedded/hal ?

@therealprof
Copy link
Contributor

Looks mostly okay to me. I do find the "rewrapping" a bit weird.

Can't we just do:

...
            match $e {
                Err($crate::Error::Other(_)) => {
                    #[allow(unreachable_code)]
                    break $e
                },
...

@eldruin
Copy link
Member

eldruin commented Jul 5, 2020

Looks mostly okay to me. I do find the "rewrapping" a bit weird.

I agree. Good point.
@therealprof: $e is a Result, you probably meant:

match $e {
    Err($crate::Error::WouldBlock) => {
        if !$c {
            break Err($crate::Error::WouldBlock);
        }
    },
    Err(e) => {
        #[allow(unreachable_code)]
        break Err(e)
    },
    Ok(x) => break Ok(x),
}

@kellerkindt Would you do this change, rebase to master and add an example to the documentation?

@therealprof
Copy link
Contributor

@therealprof: $e is a Result, you probably meant:

Nope, I meant $e, as long as you're not moving the item whatever you're matching on (if in doubt, use a reference), you can just reuse it. Not sure whether that's not possible in case due to macro hygiene or something...

@eldruin
Copy link
Member

eldruin commented Jul 5, 2020

@therealprof: $e is a Result, you probably meant:

Nope, I meant $e, as long as you're not moving the item whatever you're matching on (if in doubt, use a reference), you can just reuse it. Not sure whether that's not possible in case due to macro hygiene or something...

Interesting! I assumed the matching branch would move it and thus drop it on _ but I tried it and it works just fine. Thanks!
@kellerkindt please disregard my comment.

@kellerkindt kellerkindt requested a review from a team as a code owner November 21, 2020 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants